Add eBPF connection tracking #2

asymmetric · 2016-10-28T13:11:27Z

Introduces connection tracking via eBPF. This allows scope to get notified of every connection event, without relying on the parsing of /proc/net, and therefore:

improve performance
increase reliability

How it works

...

Limitations:

Does not support IPv6
Sets procspied: true on connections coming from eBPF
Reads 5000 bytes of /proc/net/tcp, whereas the actual file could be larger

alban · 2016-10-28T13:17:15Z

You should have a better commit message of course, explain the purpose, how it works, limitations ;)

and for the final PR, mentions which bug it fixes: 1168, possibly 1260, etc.

alban · 2016-10-28T13:18:35Z

Limitations:

IPv6
add yours here...

alban · 2016-10-28T13:21:14Z

docker/tcpv4tracer.py

+#
+# tcpv4tracer   Trace TCP IPv4 connections.
+#               For Linux, uses BCC, eBPF. Embedded C.
+#


add a reference of similar tools on the bcc repo: iovisor/bcc#762

What kind of reference do you mean exactly? BTW this whole file, including this section, is going to change with @iaguis's new tracer.

Something like:

# tcpv4tracer is similar to https://github.com/iovisor/bcc/pull/762

alban · 2016-10-28T13:22:42Z

probe/endpoint/ebpf/main.go

+)
+
+func main() {
+	tr := endpoint.NewEbpfTracker("/home/asymmetric/code/kinvolk/bcc/examples/tracing/tcpv4tracer.py")


hardcoded path.

This file should be converted to Go tests and add some real tests.

alban · 2016-10-28T13:24:30Z

docker/tcpv4tracer.py

+parser = argparse.ArgumentParser(
+    description="Trace TCP IPv4 connections",
+    formatter_class=argparse.RawDescriptionHelpFormatter)
+parser.add_argument("-p", "--pid",


the --pid option is not used in Scope. Do you want to keep it to keep close to the bcc or to remove it?

alban · 2016-10-28T13:32:12Z

docker/tcpv4tracer.py

+#define TCP_EVENT_TYPE_ACCEPT  2
+#define TCP_EVENT_TYPE_CLOSE   3
+
+struct tcp_event_t {


I see that we diverge from bcc here. So you could remove --pid too I guess..

alban · 2016-10-28T13:32:56Z

docker/tcpv4tracer.py

+                return 0;
+        }
+
+


double empty line

alban · 2016-10-28T13:39:06Z

probe/endpoint/ebpf.go

+func (n nilTracker) initialize()                                       {}
+func (n nilTracker) isInitialized() bool                               { return false }
+
+// EbpfTracker contains the list of eBPF events, and the eBPF script's command


the list of eBPF events

It is no longer the list of eBPF events but the set of IPv4 TCP connections.

the eBPF script's command

That comment comes from when the script path was in the struct. It is not true anymore.

alban · 2016-10-28T13:40:59Z

probe/endpoint/ebpf.go

+	}()
+
+	reader := bufio.NewReader(stdout)
+	// skip fist line


s/fist/first

either explain why you skip the first line or just remove that, add a --no-header option to the python script so that it does not print the header.

alban · 2016-10-28T13:45:25Z

probe/endpoint/four_tuple.go

+}
+
+// key is a sortable direction-independent key for tuples, used to look up a
+// fourTuple, when you are unsure of it's direction.


asymmetric · 2016-10-28T13:47:30Z

probe/endpoint/ebpf.go

+	initialized bool
+	dead        bool
+
+	activeFlows   map[string]ebpfConnection


@alban I get the point of trying to mirror the conntrack.go module, but would it make more sense to just say activeConnections, walkConnections,... ?

sounds ok to me.

alban · 2016-10-28T13:48:57Z

probe/process/walker.go

-// Walker, and then allows other concurrent readers to Walk that copy.
+// CachingWalker is a walker that wraps a platform-specific walker (source),
+// triggers it at every Tick() and caches a copy of the output, to allow
+// other concurrent readers to Walk that copy.


The changes in this file belong to another PR: they are unrelated to the ebpf work.

asymmetric · 2016-10-28T13:50:46Z

probe/endpoint/ebpf.go

+	dead        bool
+
+	activeFlows   map[string]ebpfConnection
+	bufferedFlows []ebpfConnection


I would also call this closedConnections, much more descriptive.

@alban, what do you think?

ok

Can you add a comment saying that they are recently closed connections and they are not staying forever here but only until the next walkConnections

alban · 2016-10-28T13:50:47Z

prog/probe.go

 		p.AddTicker(processCache)
+		// if eBPF tracking is enabled, scan /proc synchronously, and just once


Note that if eBPF tracking is enabled but does not work (old kernel, or no kernel header or whatever), we need to fall back correctly to the old connection scanner (the asynchronous one)...

alban · 2016-10-28T13:52:54Z

prog/probe.go

@@ -158,6 +164,7 @@ func probeMain(flags probeFlags, targets []appclient.Target) {
 		SpyProcs:     flags.spyProcs,
 		UseConntrack: flags.useConntrack,
 		WalkProc:     flags.procEnabled,
+		EbpfEnabled:  flags.ebpfEnabled,


we could track several things with ebpf: processes, connection, ios...

but this PR is only about tracking connections. We should have a better name for the option than a generic "ebpf", we should mention "connections" somewhere.

Ditto for the "EbpfTracker"

asymmetric · 2016-10-28T14:26:05Z

probe/endpoint/four_tuple.go

+	t.fromAddr, t.fromPort, t.toAddr, t.toPort = t.toAddr, t.toPort, t.fromAddr, t.fromPort
+}
+
+// reverse flips the direction of a tuple, without side effects


@alban do you think it makes sense to have both the function and the method?

if they are used, yes. One is more demanding on the GC than the other.

Right, but I could use just the method (which I suppose is less demanding on the GC). I only created the function because I think it looks better :)

asymmetric · 2016-10-28T14:32:05Z

probe/endpoint/ebpf.go

+}
+
+func (t *EbpfTracker) initialize() {
+	t.initialized = true


@alban do you think it's good practice to wrap this in a mutex?

this does not seem necessary: t.initialized is only read & written from the reporter thread and not from the ebpf tracker "run()" thread.

alban · 2016-10-28T15:29:29Z

Can you add in the PR the list of tests you did:

✅ get initial connections
✅ get connections from NAT (tested with Kubernetes Services)
❌ falling back to the old parsing method when ebpf does not work at all (old kernel)
❌ falling back to the old parsing method when ebpf stops working (python script crashed)
❌ IPv6
❓ connection between 2 containers using the same network namespace
✅ displaying connection in the container view when both the source and the destination are pods with multiple containers

alepuccetti · 2017-01-17T12:52:33Z

We are making the official one on scope, don't need this anymore

eBPF

11f4f6a

alban reviewed Oct 28, 2016

View reviewed changes

docker/tcpv4tracer.py

return 0;

}

Copy link

alban Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double empty line

alban reviewed Oct 28, 2016

View reviewed changes

asymmetric commented Oct 28, 2016

View reviewed changes

alban reviewed Oct 28, 2016

View reviewed changes

asymmetric commented Oct 28, 2016

View reviewed changes

alban reviewed Oct 28, 2016

View reviewed changes

Lorenzo Manacorda added 4 commits October 28, 2016 15:56

document ebpf.go

038d2c1

Reset changes in probe/process/walker.go

30f2466

rename flows to connections

dcf0fe9

fix comments

81753cc

asymmetric commented Oct 28, 2016

View reviewed changes

asymmetric changed the title ~~eBPF~~ Add eBPF connection tracking Oct 28, 2016

Lorenzo Manacorda added 2 commits October 28, 2016 16:55

make ebpf flag specific to connections

c26da77

fix leftover flag changes

182f3db

alepuccetti closed this Jan 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add eBPF connection tracking #2

Add eBPF connection tracking #2

asymmetric commented Oct 28, 2016 •

edited

Loading

alban commented Oct 28, 2016

alban commented Oct 28, 2016

alban Oct 28, 2016

asymmetric Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

asymmetric Oct 28, 2016 •

edited

Loading

alban Oct 28, 2016

alban Oct 28, 2016

asymmetric Oct 28, 2016

asymmetric Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

alban Oct 28, 2016

asymmetric Oct 28, 2016

alban Oct 28, 2016

asymmetric Oct 28, 2016

asymmetric Oct 28, 2016

alban Oct 28, 2016

alban commented Oct 28, 2016 •

edited

Loading

alepuccetti commented Jan 17, 2017

		p.AddTicker(processCache)
		// if eBPF tracking is enabled, scan /proc synchronously, and just once

Add eBPF connection tracking #2

Add eBPF connection tracking #2

Conversation

asymmetric commented Oct 28, 2016 • edited Loading

How it works

Limitations:

alban commented Oct 28, 2016

alban commented Oct 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asymmetric Oct 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alban commented Oct 28, 2016 • edited Loading

alepuccetti commented Jan 17, 2017

asymmetric commented Oct 28, 2016 •

edited

Loading

asymmetric Oct 28, 2016 •

edited

Loading

alban commented Oct 28, 2016 •

edited

Loading